fix(session): require valid JWT expiration - #442
Merged
Merged
Conversation
PKCEHelper's four flows (AuthKit/SSO authorization URL and code exchange) required callers to re-pass a clientId the WorkOS client already carries (constructor arg / WORKOS_CLIENT_ID). Make the parameter optional with a fallback to requireClientId(), aligning PHP with the other backend SDKs' override-with-fallback pattern. Explicit arguments still win; an unconfigured client now throws ConfigurationException instead of a TypeError. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed session tokens without a numeric expiration could bypass expiry validation indefinitely. Fail closed while preserving numeric-string compatibility, and reject tokens at the expiration boundary. Addresses VULN-1270.
Contributor
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Casting exp to int before the boundary check truncated fractional NumericDate values, so a token expiring at 1700000000.5 was rejected at 1700000000 even though it was still valid. Compare as float instead and cover both sides of the fractional boundary in the expiration matrix.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
expis missing, null, or non-numeric, preventing signed session tokens from bypassing expiration checks.exp == time()boundary, while preserving numeric-string compatibility.invalid_jwtresponse.iss/aud/nbfvalidation is intentionally deferred. Issuer validation is tracked separately in feat: Add optional issuer check to SessionManager authenticate #440, and the existing issuer/audience TODO remains unchanged.Validation
composer cipasses: PHP-CS-Fixer reports 0 fixable files, PHPStan reports 0 errors, and PHPUnit reports 366 tests and 1,708 assertions. The two original missing-fixture skips are pre-existing and unrelated; currentmainadds a third (OrganizationsTest::testListItContacts). All 15 expiration cases pass.Current
mainwas merged without conflicts to exclude an inherited, already-merged PKCE change from the PR diff. Onlylib/SessionManager.php,tests/SessionManagerTest.php, andtests/Fixtures/session_expiration_clock.phpdiffer frommain.